app/Audit/ConcreteFormatters/SummitAttendeeAuditLogFormatter.php (2)

118-123: Remove extraneous blank lines.

Lines 119-121 contain unnecessary whitespace that appears to be a leftover from editing.

🧹 Proposed fix

         try {
             $removed_ids = $collectionData['removed_ids'] ?? [];
-
-            
-
             $field = $collectionData['field'] ?? 'unknown';

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@app/Audit/ConcreteFormatters/SummitAttendeeAuditLogFormatter.php` around
lines 118 - 123, Remove the extraneous blank lines between the assignment of
$removed_ids and the subsequent assignments to $field and $targetEntity in the
SummitAttendeeAuditLogFormatter class so the code reads compactly as consecutive
assignments (ensure $removed_ids = $collectionData['removed_ids'] ?? []; is
immediately followed by $field = $collectionData['field'] ?? 'unknown'; and
$targetEntity = $collectionData['target_entity'] ?? 'unknown';).

---

59-76: Remove unused $subject parameter.

The $subject parameter is declared but never used within handleManyToManyCollection. Since $id and $name are already extracted and passed separately, this parameter is redundant.

♻️ Proposed fix

-    private function handleManyToManyCollection(SummitAttendee $subject, array $change_set, $id, $name, bool $isDeletion): ?string
+    private function handleManyToManyCollection(array $change_set, $id, $name, bool $isDeletion): ?string
     {

Also update the call sites in lines 47 and 50:

                 case IAuditStrategy::EVENT_COLLECTION_MANYTOMANY_UPDATE:
-                    return $this->handleManyToManyCollection($subject, $change_set, $id, $name, false);
+                    return $this->handleManyToManyCollection($change_set, $id, $name, false);

                 case IAuditStrategy::EVENT_COLLECTION_MANYTOMANY_DELETE:
-                    return $this->handleManyToManyCollection($subject, $change_set, $id, $name, true);
+                    return $this->handleManyToManyCollection($change_set, $id, $name, true);

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@app/Audit/ConcreteFormatters/SummitAttendeeAuditLogFormatter.php` around
lines 59 - 76, The method handleManyToManyCollection has an unused $subject
parameter; remove $subject from its signature (change function
handleManyToManyCollection(SummitAttendee $subject, array $change_set, $id,
$name, bool $isDeletion): ?string to handleManyToManyCollection(array
$change_set, $id, $name, bool $isDeletion): ?string) and update all call sites
that pass the subject (the two places invoking handleManyToManyCollection) to
stop providing the extra argument so they pass only $change_set, $id, $name,
$isDeletion; ensure type hints and usages inside handleManyToManyCollection
remain consistent after removal.

app/Audit/ConcreteFormatters/DefaultEntityManyToManyCollectionUpdateAuditLogFormatter.php (1)

55-56: Consider: Inconsistent key naming between added_ids/removed_ids and deleted_ids.

The code checks for both 'removed_ids' and 'deleted_ids' as fallback. While this provides flexibility, it may lead to confusion about which key should be used. Consider documenting the expected payload structure or standardizing on one key name.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@app/Audit/ConcreteFormatters/DefaultEntityManyToManyCollectionUpdateAuditLogFormatter.php`
around lines 55 - 56, Standardize the change_set keys by treating "removed_ids"
as the canonical name and explicitly map any legacy "deleted_ids" to it before
use in DefaultEntityManyToManyCollectionUpdateAuditLogFormatter: in the code
that prepares $change_set (or immediately before computing
$addedIds/$removedIds) normalize $change_set['removed_ids'] =
$change_set['removed_ids'] ?? $change_set['deleted_ids'] ?? null, then set
$addedIds = $change_set['added_ids'] ?? null and $removedIds =
$change_set['removed_ids']; also update any docblocks/comments or
function/method docs that reference $change_set to state that "removed_ids" is
the standard key (and "deleted_ids" is supported as an alias).

app/Audit/ConcreteFormatters/DefaultEntityManyToManyCollectionDeleteAuditLogFormatter.php (1)

60-65: Debug logging may be verbose in production.

The debug logging includes the full deletedIds array content. For collections with many IDs, this could produce very large log entries. Consider logging only the count in debug mode.

🔧 Suggested improvement

             $deletedIds = isset($change_set['deleted_ids']) ? $change_set['deleted_ids'] : null;
             Log::debug("DefaultEntityManyToManyCollectionDeleteAuditLogFormatter::format - deletedIds", [
-                'deletedIds' => $deletedIds,
                 'isNull' => $deletedIds === null,
                 'count' => $deletedIds ? count($deletedIds) : 0
             ]);

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@app/Audit/ConcreteFormatters/DefaultEntityManyToManyCollectionDeleteAuditLogFormatter.php`
around lines 60 - 65, The debug statement in
DefaultEntityManyToManyCollectionDeleteAuditLogFormatter::format currently logs
the entire $deletedIds array which can be huge; change the Log::debug call to
avoid serializing the full array and instead log only the count (use
count($deletedIds) or 0 when null) and a short flag indicating null/empty,
referencing the $deletedIds variable so the formatter still provides useful
diagnostic info without dumping the full collection.

app/Audit/AbstractAuditLogFormatter.php (1)

189-239: Edge case: Empty message detail when no IDs present.

When both $addedIds and $removedIds are empty arrays, formatManyToManyDetailedMessage will produce a message ending with a colon and empty detail: "Many-to-Many collection 'field' action: ". Consider handling this edge case.

🔧 Suggested improvement

     protected static function formatManyToManyDetailedMessage(array $details, int $addCount, int $removeCount, string $action): string
     {
         $field = $details['field'] ?? 'unknown';
         $target = $details['target_entity'] ?? 'unknown';
         $addedIds = $details['added_ids'] ?? [];
         $removedIds = $details['removed_ids'] ?? [];

         $parts = [];
         if (!empty($addedIds)) {
             $parts[] = sprintf("Added %d %s(s): %s", $addCount, $target, implode(', ', $addedIds));
         }
         if (!empty($removedIds)) {
             $parts[] = sprintf("Removed %d %s(s): %s", $removeCount, $target, implode(', ', $removedIds));
         }

         $detailStr = implode(' | ', $parts);
+        if (empty($detailStr)) {
+            return sprintf("Many-to-Many collection '%s' %s: no changes", $field, $action);
+        }
         return sprintf("Many-to-Many collection '%s' %s: %s", $field, $action, $detailStr);
     }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@app/Audit/AbstractAuditLogFormatter.php` around lines 189 - 239,
formatManyToManyDetailedMessage can produce a trailing colon with empty details
when both added and removed ID lists are empty; change the method to detect when
$parts is empty and return a cleaner message (e.g., "Many-to-Many collection
'FIELD' ACTION: no changes" or "Many-to-Many collection 'FIELD' ACTION" without
the colon). Update the logic in formatManyToManyDetailedMessage to check if
$parts is empty and set $detailStr to a sensible placeholder (or omit the colon)
before building the final sprintf; reference the method name
formatManyToManyDetailedMessage and the keys 'field', 'added_ids', 'removed_ids'
to locate the code to modify. Ensure unit/output formatting remains consistent
with existing messages.

app/Audit/AuditLogOtlpStrategy.php (1)

200-209: Minor: Inconsistent error message format.

The error message at line 209 has an unusual format with "AuditLogOtlpStrategy:: unknown targetEntity" (double colon followed by space). Consider using a consistent format.

🔧 Suggested fix

         } catch (\Exception $ex) {
-            return 'AuditLogOtlpStrategy:: unknown targetEntity';
+            return 'unknown';
         }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@app/Audit/AuditLogOtlpStrategy.php` around lines 200 - 209, The catch block
in AuditLogOtlpStrategy that currently returns "AuditLogOtlpStrategy:: unknown
targetEntity" should be changed to a consistently formatted message (for example
"AuditLogOtlpStrategy: unknown targetEntity") and preferably include the
exception details; update the return in the catch of the method that calls
$collection->getMapping() to return a string like "AuditLogOtlpStrategy: unknown
targetEntity - " . $ex->getMessage() (or at minimum remove the double colon and
extra space so it reads "AuditLogOtlpStrategy: unknown targetEntity").

tests/OpenTelemetry/Formatters/AuditEventListenerTest.php (1)

66-122: Add listener coverage for initialized collection deletions.

AuditEventListener::auditCollection() now has a separate delete path for initialized many-to-many collections via fetchDeletedIdsFromInitializedCollection(), but this file only exercises the non-many-to-many branch and the raw ID fetch helper. That leaves the new diff-based deleted_ids payload untested at the listener level. PersistentCollectionTestHelper should make this path straightforward to cover.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@tests/OpenTelemetry/Formatters/AuditEventListenerTest.php` around lines 66 -
122, Add a new unit test that exercises AuditEventListener::auditCollection()'s
many-to-many delete branch by constructing an initialized PersistentCollection
(use PersistentCollectionTestHelper to create a collection that is initialized
and has removed elements), attach it to a mock owner and mapping for a
ManyToManyOwningSideMapping, inject a mocked EntityManager (and ClassMetadata)
into the AuditEventListener instance, then call auditCollection() and assert the
emitted payload includes the diff-style "deleted_ids" (e.g., [10,11]) produced
by AuditEventListener::fetchDeletedIdsFromInitializedCollection(); ensure the
test uses the same mocking pattern as existing tests (QueryBuilder/Connection
mocks if needed) and targets the listener-level behavior rather than the helper
method directly.

app/Audit/AbstractAuditLogFormatter.php (1)

102-104: Minor indentation inconsistency.

Line 103 has extra indentation that doesn't match the surrounding code style.

🧹 Suggested fix

     protected function getUserInfo(): string
     {
         if (app()->runningInConsole()) {
-                return 'Worker Job';
+            return 'Worker Job';
         }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@app/Audit/AbstractAuditLogFormatter.php` around lines 102 - 104, In
AbstractAuditLogFormatter (the method containing the app()->runningInConsole()
check) there is an indentation mismatch: the line with "return 'Worker Job';" is
over-indented; adjust the indentation to match the surrounding block style so
the return aligns with the if block's opening brace and other statements in the
method.

app/Audit/ConcreteFormatters/SummitAttendeeAuditLogFormatter.php (1)

114-117: Remove extraneous blank lines.

Lines 115-117 contain unnecessary whitespace that should be cleaned up for consistency.

🧹 Suggested fix

         try {
             $removed_ids = $collectionData['removed_ids'] ?? [];
-
-            
-
             $field = $collectionData['field'] ?? 'unknown';

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@app/Audit/ConcreteFormatters/SummitAttendeeAuditLogFormatter.php` around
lines 114 - 117, In SummitAttendeeAuditLogFormatter, remove the extra blank
lines after the $removed_ids = $collectionData['removed_ids'] ?? []; assignment
so the assignment sits directly adjacent to the next statement; locate the
$removed_ids variable usage in this method (and the surrounding $collectionData
reference) and collapse the stray whitespace to match the project's whitespace
style.

tests/OpenTelemetry/Formatters/SummitAttendeeAuditLogFormatterManyToManyTest.php (1)

71-79: ⚠️ Potential issue | 🟡 Minor

“Without context” rows still don’t hit the no-context branch.

On Lines 71-79, all provider cases pass [], so provider rows on Lines 206-207 effectively test missing collection (same as Lines 208-209) instead of “collection present but context missing”.

🧪 Proposed test fix

     public function testManyToManyReturnsNullWithoutRequiredContextOrCollection(
         string $eventType,
-        bool $withContext
+        bool $withContext,
+        bool $withCollection
     ): void {
         $formatter = $this->makeFormatter($eventType, $withContext);
         $attendee = $this->makeAttendee();
-
-        $result = $formatter->format($attendee, []);
+        $changeSet = $withCollection
+            ? ['collection' => $this->makeCollection(self::SNAPSHOT_IDS_EMPTY, self::CURRENT_IDS_EMPTY)]
+            : [];
+        $result = $formatter->format($attendee, $changeSet);
         $this->assertNull($result);
     }
@@
-            self::DP_UPDATE_WITHOUT_CONTEXT => [IAuditStrategy::EVENT_COLLECTION_MANYTOMANY_UPDATE, false],
-            self::DP_DELETE_WITHOUT_CONTEXT => [IAuditStrategy::EVENT_COLLECTION_MANYTOMANY_DELETE, false],
-            self::DP_UPDATE_WITHOUT_COLLECTION => [IAuditStrategy::EVENT_COLLECTION_MANYTOMANY_UPDATE, true],
-            self::DP_DELETE_WITHOUT_COLLECTION => [IAuditStrategy::EVENT_COLLECTION_MANYTOMANY_DELETE, true],
+            self::DP_UPDATE_WITHOUT_CONTEXT => [IAuditStrategy::EVENT_COLLECTION_MANYTOMANY_UPDATE, false, true],
+            self::DP_DELETE_WITHOUT_CONTEXT => [IAuditStrategy::EVENT_COLLECTION_MANYTOMANY_DELETE, false, true],
+            self::DP_UPDATE_WITHOUT_COLLECTION => [IAuditStrategy::EVENT_COLLECTION_MANYTOMANY_UPDATE, true, false],
+            self::DP_DELETE_WITHOUT_COLLECTION => [IAuditStrategy::EVENT_COLLECTION_MANYTOMANY_DELETE, true, false],
         ];
     }

Also applies to: 203-210

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@tests/OpenTelemetry/Formatters/SummitAttendeeAuditLogFormatterManyToManyTest.php`
around lines 71 - 79, The test
testManyToManyReturnsNullWithoutRequiredContextOrCollection is passing an empty
array for the formatter context for all provider cases, so it never exercises
the "collection present but context missing" branch; update the data provider
used by makeFormatter to supply two separate cases: one where the second
argument is an empty array (both collection and context missing) and one where a
collection key (matching what SummitAttendeeAuditLogFormatter expects) is
present but the required context is omitted (e.g., pass ['collection' =>
$someCollection] or the formatter's actual collection key) so the
formatter->format($attendee, $context) call tests the "collection present but
context missing" path as well; adjust the provider rows that feed
testManyToManyReturnsNullWithoutRequiredContextOrCollection and any other
affected tests (e.g., the provider used by the test at lines noted) to include
these distinct cases.

app/Audit/AuditLogOtlpStrategy.php (1)

190-203: ⚠️ Potential issue | 🟡 Minor

Handle explicitly provided empty deleted_ids distinctly from “missing”.

On Line 190, !empty($change_set['deleted_ids']) treats [] as absent and falls back to counting the collection, which can misreport delete audit counts. Use key-existence + array validation instead.

🐛 Proposed fix

-                    if (!empty($change_set['deleted_ids'])) {
+                    if (array_key_exists('deleted_ids', $change_set) && is_array($change_set['deleted_ids'])) {
                         $data['audit.collection_count'] = count($change_set['deleted_ids']);
                         if ($collection->isInitialized()) {
                             $changes = $this->getCollectionChanges($collection, $change_set);
                             $data['audit.collection_current_count'] = $changes['current_count'];
                             $data['audit.collection_snapshot_count'] = $changes['snapshot_count'];
                             $data['audit.collection_is_dirty'] = $changes['is_dirty'] ? 'true' : 'false';
                         } else {
                             $data['audit.collection_current_count'] = 0;
                             $data['audit.collection_snapshot_count'] = 0;
                             $data['audit.collection_is_dirty'] = 'true';
                         }
                     } elseif ($collection->isInitialized()) {

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@app/Audit/AuditLogOtlpStrategy.php` around lines 190 - 203, Replace the
!empty($change_set['deleted_ids']) check so an explicitly provided empty array
is treated as "present" instead of missing: use array_key_exists('deleted_ids',
$change_set) && is_array($change_set['deleted_ids']) to detect the key, then set
$data['audit.collection_count'] = count($change_set['deleted_ids']) (which will
be 0 for an empty array) and keep the subsequent logic that calls
getCollectionChanges($collection, $change_set) when
$collection->isInitialized(), otherwise set the current/snapshot counts and
is_dirty as before; update the condition that falls back to counting the
collection to only run when the deleted_ids key is absent.

app/Audit/AuditEventListener.php (1)

115-136: ⚠️ Potential issue | 🟡 Minor

uiApp and uiFlow are permanently null in this path.

$ui is initialized to an empty array and never populated, so Lines 135-136 cannot emit any UI context. If dropping those fields was intentional, pass null directly and remove the dead variable; otherwise restore the request-derived extraction here.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@app/Audit/AuditEventListener.php` around lines 115 - 136, The uiApp/uiFlow
fields are always null because $ui is initialized as an empty array and never
populated; either remove $ui and pass nulls directly to AuditContext or restore
extraction from the Request before returning. Update the code in
AuditEventListener (around the request handling and return new
AuditContext(...)) to pull UI info from the request (e.g., decode a JSON
payload/header/session value into $ui and set uiApp/uiFlow) or replace uiApp:
$ui['app'] ?? null and uiFlow: $ui['flow'] ?? null with explicit nulls and
remove the dead $ui variable and any unused extraction logic. Ensure you
reference the AuditContext constructor and the uiApp/uiFlow parameters when
making the change.

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

app/Audit/AuditLogOtlpStrategy.php (1)

193-206: ⚠️ Potential issue | 🟡 Minor

Treat deleted_ids as explicit input even when it's empty.

fetchManyToManyIds() can legitimately return []. Using !empty(...) collapses that into “missing” and falls back to count($collection), which can skew audit.collection_count for empty-delete cases.

🐛 Proposed fix

-                    if (!empty($change_set['deleted_ids'])) {
+                    if (array_key_exists('deleted_ids', $change_set) && is_array($change_set['deleted_ids'])) {
                         $data['audit.collection_count'] = count($change_set['deleted_ids']);

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@app/Audit/AuditLogOtlpStrategy.php` around lines 193 - 206, The code
currently uses !empty($change_set['deleted_ids']) which treats an explicit empty
array as "missing" and wrongly falls back to count($collection); change the
condition to check for the presence of the key (e.g.
array_key_exists('deleted_ids', $change_set) or
isset($change_set['deleted_ids'])) so that an explicit [] from
fetchManyToManyIds() is honored, and when the key exists set
audit.collection_count = count($change_set['deleted_ids']) and keep the existing
logic that computes current/snapshot/is_dirty via
getCollectionChanges($collection, $change_set) or the else branch when the
collection is not initialized.

tests/OpenTelemetry/Formatters/PresentationCategoryGroupAuditLogFormatterManyToManyTest.php (1)

63-72: ⚠️ Potential issue | 🟡 Minor

The “without context” rows still never pass a collection.

testManyToManyReturnsNullWithoutRequiredContextOrCollection() always calls format($group, []), so the provider rows named “without context” still only exercise the missing-collection path. Add a withCollection flag and pass a real collection for those rows.

🧪 Proposed fix

     public function testManyToManyReturnsNullWithoutRequiredContextOrCollection(
         string $eventType,
-        bool $withContext
+        bool $withContext,
+        bool $withCollection
     ): void {
         $formatter = $this->makeFormatter($eventType, $withContext);
         $group = $this->makeGroup();
-
-        $result = $formatter->format($group, []);
+        $changeSet = $withCollection
+            ? ['collection' => $this->makeCollection(self::SNAPSHOT_IDS_EMPTY, self::CURRENT_IDS_EMPTY)]
+            : [];
+        $result = $formatter->format($group, $changeSet);
         $this->assertNull($result);
     }
@@
-            self::DP_UPDATE_WITHOUT_CONTEXT => [IAuditStrategy::EVENT_COLLECTION_MANYTOMANY_UPDATE, false],
-            self::DP_DELETE_WITHOUT_CONTEXT => [IAuditStrategy::EVENT_COLLECTION_MANYTOMANY_DELETE, false],
-            self::DP_UPDATE_WITHOUT_COLLECTION => [IAuditStrategy::EVENT_COLLECTION_MANYTOMANY_UPDATE, true],
-            self::DP_DELETE_WITHOUT_COLLECTION => [IAuditStrategy::EVENT_COLLECTION_MANYTOMANY_DELETE, true],
+            self::DP_UPDATE_WITHOUT_CONTEXT => [IAuditStrategy::EVENT_COLLECTION_MANYTOMANY_UPDATE, false, true],
+            self::DP_DELETE_WITHOUT_CONTEXT => [IAuditStrategy::EVENT_COLLECTION_MANYTOMANY_DELETE, false, true],
+            self::DP_UPDATE_WITHOUT_COLLECTION => [IAuditStrategy::EVENT_COLLECTION_MANYTOMANY_UPDATE, true, false],
+            self::DP_DELETE_WITHOUT_COLLECTION => [IAuditStrategy::EVENT_COLLECTION_MANYTOMANY_DELETE, true, false],
         ];
     }

Also applies to: 147-154

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@tests/OpenTelemetry/Formatters/PresentationCategoryGroupAuditLogFormatterManyToManyTest.php`
around lines 63 - 72, The test always passes an empty array as the collection
context, so rows meant to exercise the “without context” case never provide a
real collection; update the data provider providesNullCasesForManyToMany to
return a third boolean flag (e.g., withCollection) and change
testManyToManyReturnsNullWithoutRequiredContextOrCollection signature to accept
bool $withCollection, then when $withCollection is true pass a real collection
(e.g., $this->makeCollection()) to $formatter->format($group, $collection) and
otherwise pass [] as before; apply the same change to the other similar
test/provider pair around lines 147-154 so both tests correctly exercise
missing-context vs missing-collection paths.

app/Audit/AuditEventListener.php (1)

115-136: ⚠️ Potential issue | 🟡 Minor

uiApp and uiFlow are still guaranteed to be null.

$ui is initialized as [] and never populated, so Lines 135-136 always emit null. Either hydrate these values from request context or stop threading them through AuditContext here.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@app/Audit/AuditEventListener.php` around lines 115 - 136, The $ui array is
initialized but never populated so AuditContext fields uiApp and uiFlow (set at
uiApp and uiFlow) will always be null; fix by extracting the UI payload from the
current request before returning AuditContext (e.g. read
$req->attributes->get('ui') or $req->get('ui') or decode a known header like
'X-UI' and set $ui['app'] and $ui['flow'] accordingly), then pass those
populated values into the AuditContext constructor, or if no UI is available
remove uiApp/uiFlow from AuditContext here; update the code around
request()/$req and AuditContext usage to use the actual source you choose.

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes